[portsorch]: Add support of cable breakout feature#287
[portsorch]: Add support of cable breakout feature#287volodymyrsamotiy wants to merge 3 commits intosonic-net:v1.0.3from volodymyrsamotiy:v1.0.3
Conversation
|
@volodymyrsamotiy, |
stcheng
left a comment
stcheng
left a comment
There was a problem hiding this comment.
Could you also change the previous ConfigDone to PortInitDone notification? That would make more sense.
orchagent/portsorch.cpp
Outdated
|
|
||
| m_portListLaneMap[lane_set] = port_id; | ||
|
|
||
| SWSS_LOG_INFO("Create port %lx with the speed %u", port_id, speed); |
There was a problem hiding this comment.
Change to notice? same as line 509.
orchagent/portsorch.h
Outdated
| bool m_portConfigDone = false; | ||
| sai_uint32_t m_portCount; | ||
| map<set<int>, sai_object_id_t> m_portListLaneMap; | ||
| map<set<int>, tuple<string, uint32_t>> m_lanesALiasSpeedMap; |
There was a problem hiding this comment.
rename to m_lanesAliasSpeedMap.
portsyncd/portsyncd.cpp
Outdated
| iss >> entry[column]; | ||
| } | ||
|
|
||
| /* If port has no alias, then use its' name as alias */ |
orchagent/portsorch.cpp
Outdated
| } | ||
| } | ||
|
|
||
| bool PortsOrch::createPort(const set<int> &lane_set, uint32_t speed) |
There was a problem hiding this comment.
change the name to addPort? it then aligns all the other add/remove functions in SwSS.
* Fix comments
orchagent/portsorch.cpp
Outdated
| if (m_lanesALiasSpeedMap.find(it->first) == m_lanesALiasSpeedMap.end()) | ||
| { | ||
| SWSS_LOG_INFO("Port has already been initialized before alias:%s", alias.c_str()); | ||
| if (!removePort(it->first)) |
There was a problem hiding this comment.
Maybe it's better to redefine the input parameter of removePort. The function removePort can use port object id directly.
* Fix comments
|
How did you test this and on what platforms? If you couldn't unit test on all supported platforms, then you should introduce automated tests to take care of that and make sure the feature works prior to committing. |
|
@Nikos-Li |
|
@Nikos-Li, they will make sure the feature works for their platform. other platform owners will need to test this on their platform. Since some platform do not support certain SAI function calls, we need to make sure this feature would not break (ensure it will not be invoked on those platforms.) |
|
@volodymyrsamotiy At minimum, you should include output from your testing and since you don't have access to other platforms and use the google unit test framework to introduce tests that exercise the code and functionality you added in the orch agent. |
|
@lguohan The code in orch agent is platform independent and not under the responsibility of any platform owners. To make sure that the feature doesn't break and the code is working properly, if automated framework tests are not acceptable, then google unit test should be introduced. |
stcheng
left a comment
stcheng
left a comment
There was a problem hiding this comment.
could you generate a new pull request against the master?
* Add support for FDB notification in virtual switch * Change aging fdb time to default * Address comments * Address comments * Address comments * Change notification to debug on fdb * Move sleep after processing to avoid deadlock * Remove join thread to remove possible deadlock * Bring back thread join Without join if process will call uninitialize and thread was running we will get coredump caused by not finished thread * Fix deadlock problem with try_lock
Description The heath metric in ssd_generic for innodisk SSDs is too lazy. Fix to match the entire health number rather than just the first digit. How Has This Been Tested? Manual testing on Mellanox MSN2100
6 participants
No description provided.